Promote LEDEditor to a first-class editor#779
Conversation
It now has both Qt and Wx backends, so no reason not to.
|
I notice that #778 removes the documentation for |
Rats. No it doesn't. It just moves it. Sorry for the noise. |
|
In the demo, the fields are marked "left aligned", "right aligned", etc., but I don't see any difference in the alignments. Should I?
(EDIT: this is with Python 3.8.2 / PyQt5 5.14.2 / macOS 10.14.6) |
|
Ah, I ran the demo before looking at the source, which says that alignment is wx-only. |
mdickinson
left a comment
mdickinson
left a comment
There was a problem hiding this comment.
Ah, I ran the demo before looking at the source, which says that alignment is wx-only.
Hmm. The alignment doesn't seem to work with wx (wxPython4) either:
| # is also available online at http://www.enthought.com/licenses/BSD.txt | ||
| # Thanks for using Enthought open source! | ||
|
|
||
| """ A Traits UI editor that wraps a LED-style integer display. |
| # editor names for factory to find | ||
| ReadonlyEditor = LEDEditor | ||
| SimpleEditor = LEDEditor | ||
| CustomEditor = LEDEditor |
There was a problem hiding this comment.
Do we actually need to populate all the three styles? If the custom/readonly editors are not implemented, then in the future we will be certain that they are never used because attempts to set style='custom' or style='readonly' will be met with an error (I think). If we populate them, it will add constraint to future extensions when we do want the custom / readonly style to behave completely differently.
e.g. The TableEditor does not have a custom editor. ButtonEditor does not have a readonly editor. They don't seem to be missed.
| if ETSConfig.toolkit == 'wx': | ||
| from traitsui.wx.extra.led_editor import LEDEditor | ||
| else: | ||
| from traitsui.qt4.extra.led_editor import LEDEditor |
There was a problem hiding this comment.
Shall we keep traitsui.qt4.extra.led_editor and traitsui.wx.extra.led_editor as aliases? It costs almost nothing to maintain aliases. Otherwise this would be a breaking change.
| .. automodule:: traitsui.editors.led_editor | ||
| :members: | ||
| :undoc-members: | ||
| :show-inheritance: |
There was a problem hiding this comment.
The API documentation is now being generated automatically. So this file and traitsui.editors.rst are not needed any more.
|
|
||
| def _alignment_changed(self): | ||
| if self.control is not None: | ||
| self.control.SetAlignment(LEDStyles[self.alignment]) |
There was a problem hiding this comment.
Could we make such a change in a separate PR such that this PR is more focused about exposing the existing editor as first-class / built-in / whatever editor?
Furthermore I expect there being very limited use case to mutate the alignment after the editor is created - we can wait for this feature request to arise. The fewer mutables there are, the simpler and more maintainable the code is.
| """ | ||
| self.control = LEDNumberCtrl(parent, -1) | ||
| self.control.SetAlignment(LEDStyles[self.factory.alignment]) | ||
| self.control.SetAlignment(LEDStyles[self.alignment]) |
There was a problem hiding this comment.
Same here, I think we can defer making alignment mutable in a separate PR / issue and restore the original code which uses the factory value.
(I bet one probably needs to sync_value method like this
traitsui/traitsui/qt4/list_str_editor.py
Line 114 in 7841ca7
There was a problem hiding this comment.
(I bet one probably needs to sync_value method like this...
Please ignore this half of the comment. The sync_value is used for synchronizing a trait on the editor with a trait on the object being edited. But the first half of the comment about defer making alignment mutable still stands.
3 participants
It now has both Qt and Wx backends, so no reason not to.